-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: use environment config root for hmr filename #17394
Conversation
Run & review this pull request in StackBlitz Codeflow. |
Hi @patak-dev @antfu @sheremet-va, maybe one of you will know what's happening here! With my changes the tests for the |
The test are also flaky on the v6 branch. Don't worry about that. And thanks for experimenting and giving feedback about Environment API. About making |
Hi, thanks for your answer @patak-dev!
In my experimentation, I created my environments using ssr: {
dev: {
createEnvironment: (name, config) => {
return createNodeDevEnvironment(
name,
{
...config,
root: rootDir,
// ... But yes, you're right, normally, when only specifying some overrides without creating an environment directly like above, the I make all of this hussle around Vite configurations, module loaders and even using If you would like to have some more direct example, I can try to move my Vite Env API experiment into StackBlitz. |
I still don't see why you need a different |
No, right now I need different
I spent way too much time to simplify the setup, but for what I wanted to achieve, this was the only way it worked, with multiple roots, if not, then React module resolution was not working properly. You can get a closer look at the Vite server setup at https://github.com/lazarv/react-server/blob/vite6-environments-api/packages/react-server/lib/dev/create-server.mjs#L70-L294. |
Check out https://github.com/hi-ogawa/vite-environment-examples, there are examples of doing RSC without multiple roots there. It is ok if you'd like to keep separate servers. For Environment API, I think we need to be careful in what we make per-environment. It seems it is possible to achieve what you want without separate roots, so for now, let's close this one. |
Sorry to hear! My use case is more advanced than that example, I already checked it out before. It is not possible what I want to achieve without using multiple roots. But I was very satisfied with the Environment API, so I'll resolve this issue on my side and use it as-is. Thanks for discussing this! |
Description
When using multiple environments with different
root
in the configuration, HMR emits incorrect file paths asshortName
is resolved using the main config instead of the environment's config.This PR fixes the issue by not using the resolved short filename for the
updateModules
call and using the environment config when working against a specific environment.No tests failed, Vite dev server works as expected, but HMR payload is incorrect as it contains a non-existing
triggeredBy
path.Fixes #17393